Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CRDCDH-1500 Create Approved Study List Page #468

Merged
merged 32 commits into from
Oct 2, 2024
Merged

CRDCDH-1500 Create Approved Study List Page #468

merged 32 commits into from
Oct 2, 2024

Conversation

Alejandro-Vega
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega commented Sep 18, 2024 •
edited
Loading

Overview

Create a page for listing approved studies in the Admin portal.

Change Details (Specifics)

  • Created Approved Study List route and page and modeled it after the Organization List View page
  • Fixed retrieving approved studies to sort studies server-side instead of locally
  • Added table displaying approved study data
  • Disabled sorting for Access Type until the column requirements and BE are updated
  • Provided filters to table
  • URL search parameters are turned on for this table to save table state, including the filters
  • Fixed filter bug for other tables where if you type into the input too fast then it will cause the page to crash. This was due to searchParams updating too quickly with the same values. Added a guard to prevent this.
  • Added debounce to filters to avoid excess API calls

Note

The Access Type column currently does not support sorting and is based on original requirements. This will be updated in the future PR.

Related Ticket(s)

CRDCDH-1500

@Alejandro-Vega
Update approved study types and queries
a50c94f
@Alejandro-Vega
Add route for studies and controller
8fef084
@Alejandro-Vega
Created an approved studies list context
5f5c2f0
@Alejandro-Vega
Update navbar to include new manage studies
a71bb8e
@Alejandro-Vega
Created init list view for studies and updated organization view
c25b450
@Alejandro-Vega
Merge branch 'CRDCDH-1592' of https://github.com/CBIIT/crdc-datahub-ui …
50bcd56 
…into CRDCDH-1500
@Alejandro-Vega
Add dbGaPID to listApprovedStudies query
a17e67e
@Alejandro-Vega
Update listApprovedStudies query and types
3679263
@Alejandro-Vega
Update StudiesController
fbea197
@Alejandro-Vega
Update studies list view to include filters and handling fetching app…
dc1e211 
…roved studies
@Alejandro-Vega
Remove unused context
5e59265
@Alejandro-Vega
Merge branch '3.1.0' of https://github.com/CBIIT/crdc-datahub-ui into C…
e7bfa59 
…RDCDH-1500
@Alejandro-Vega
Update type name, and add openAccess to the listApprovedStudies param…
91c073d 
…eters
@Alejandro-Vega
Remove console log
20c28a6
@Alejandro-Vega Alejandro-Vega added this to the 3.1.0 (PMVP-M2) milestone Sep 18, 2024
@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Sep 18, 2024
@coveralls
Copy link
Collaborator

coveralls commented Sep 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 11072403898

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 8 of 121 (6.61%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 46.825%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/Header/components/HeaderTabletAndMobile.tsx 1 2 50.0%
src/components/Header/components/NavbarDesktop.tsx 0 1 0.0%
src/router.tsx 0 1 0.0%
src/content/users/ProfileView.tsx 0 2 0.0%
src/content/organizations/OrganizationView.tsx 0 4 0.0%
src/content/studies/Controller.tsx 0 9 0.0%
src/content/studies/ListView.tsx 0 95 0.0%
Totals Coverage Status
Change from base Build 11053613355: -0.7%
Covered Lines: 2806
Relevant Lines: 5463
💛 - Coveralls

@Alejandro-Vega Alejandro-Vega marked this pull request as ready for review September 30, 2024 21:58
@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Sep 30, 2024
amattu2
amattu2 previously approved these changes Oct 1, 2024
View reviewed changes
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one potential action item annotated below, otherwise everything in the FE looks good to me. I'm approving it just in case no update is needed.

BTW,

The API seems to handle filtering of the "Open" access type strangely. It is excluding studies with controlledAccess + openAccess data, but if you select "Controlled", it will show you studies with controlledAccess + openAccess data still. Definitely evaluate this and see if we need to reopen the BE task or if it's expected.

src/utils/studyUtils.ts Outdated Show resolved Hide resolved
amattu2
amattu2 approved these changes Oct 2, 2024
View reviewed changes
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapproving based on clarification from KC. You can merge if there's no other changes needed.

@Alejandro-Vega Alejandro-Vega merged commit 5c8e3b1 into 3.1.0 Oct 2, 2024
5 checks passed
@Alejandro-Vega Alejandro-Vega deleted the CRDCDH-1500 branch October 2, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amattu2 amattu2 amattu2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.1.0 (PMVP-M2)
Development

Successfully merging this pull request may close these issues.
3 participants
@Alejandro-Vega @coveralls @amattu2